Skip to content

Enhanced FindComponentByInfPath for robust, case-insensitive and normalized based INF path matching to improve module lookup accuracy#787

Open
sndeb wants to merge 4 commits intotianocore:masterfrom
sndeb:master
Open

Enhanced FindComponentByInfPath for robust, case-insensitive and normalized based INF path matching to improve module lookup accuracy#787
sndeb wants to merge 4 commits intotianocore:masterfrom
sndeb:master

Conversation

@sndeb
Copy link
Copy Markdown

@sndeb sndeb commented Mar 6, 2026

Improve FindComponentByInfPath matching logic

  • Enhanced InfPath matching to normalize paths, handle both absolute and relative forms, and compare using case-insensitive, normalized equality.
  • Added controlled suffix matching: if no exact match, allow trailing path component match, but only select when unambiguous.
  • Avoids false positives by logging and returning None when multiple ambiguous matches are found.
  • Improves robustness and accuracy when locating modules by INF path.

…alized based INF path matching to improve module lookup accuracy
@sndeb sndeb changed the title Enhanced FindComponentByInfPath for robust, case-insensitive and alized based INF path matching to improve module lookup accuracy Enhanced FindComponentByInfPath for robust, case-insensitive and normalized based INF path matching to improve module lookup accuracy Mar 6, 2026
@apop5 apop5 requested review from Javagedes and spbrogan March 6, 2026 06:17
Copy link
Copy Markdown
Contributor

@Javagedes Javagedes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall this PR seems fine. I think there needs to be a bit more comments toward the latter half of the changes as you start doing random indexing and it gets a bit confusing.

You also need to write new test cases to cover this extra functionality. Not only does it ensure it works as expected but it can help prevent regressions.

Comment thread edk2toollib/uefi/edk2/parsers/buildreport_parser.py Outdated
Comment thread edk2toollib/uefi/edk2/parsers/buildreport_parser.py
Comment thread edk2toollib/uefi/edk2/parsers/buildreport_parser.py Outdated
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 11, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.10%. Comparing base (ea78c48) to head (6a2d711).
⚠️ Report is 6 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #787   +/-   ##
=======================================
  Coverage   82.10%   82.10%           
=======================================
  Files          47       47           
  Lines        7881     7881           
=======================================
  Hits         6471     6471           
  Misses       1410     1410           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Use pathlib for consistent path normalization and consolidate exact/suffix
matching, returning only the closest unambiguous match.
@sndeb sndeb requested a review from Javagedes April 2, 2026 08:33
@Javagedes
Copy link
Copy Markdown
Contributor

@sndeb thank you for the extra inline documentation, this PR looks great.

As mentioned before, please add tests to cover this new functionality. Tests for this should probably go here: https://github.com/tianocore/edk2-pytool-library/tree/master/tests.unit/parsers

It does not look like we have unit tests for the build report parser, so please add some to cover your logic. I'll create an issue for myself to add even more on top of that. My main focus on unit tests is so we can prevent functionality regression with future changes. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants